Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --allow flag #513

Merged
merged 13 commits into from
Feb 13, 2024
Merged

Conversation

Rustin170506
Copy link
Collaborator

@Rustin170506 Rustin170506 commented Jan 29, 2024

ref #493 (comment)

Description

This pull request adds a new flag to tokio-console that allows warning lints to be selectively disabled. Additionally, you can use the value all to disable all warnings.
For example, you can use it like this: following manner: cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded"

Explanation of Changes

Added an AllowedWarnings enum to the command line interface to represent the allowed warnings. All for disable all warnings. Explicit explicitly indicates which warning can be ignored.

@Rustin170506 Rustin170506 changed the title feat: add --allow flag WIP: feat: add --allow flag Jan 29, 2024
Copy link
Collaborator Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still working on adding the --allow all support.

@Rustin170506
Copy link
Collaborator Author

Rustin170506 commented Feb 7, 2024

Tested locally:

  1. panic on main.rs with panic!("{:?}", warnings);
  2. Try:
$ cargo run -- --warn "self-wakes" --allow all
     Running `target/debug/tokio-console --warn self-wakes --allow all`
The application panicked (crashed).
Message:  []
Location: tokio-console/src/main.rs:79
  1. Another one:
$ cargo run -- --warn "self-wakes" --allow ohno
error: invalid value 'ohno' for '--allow <ALLOW_WARNINGS>...': failed to parse warning: unknown warning: ohno

For more information, try '--help'.
  1. Comma list:
$ cargo run -- --warn "self-wakes,lost-waker,never-yielded" --allow "self-wakes,never-yielded"
     Running `target/debug/tokio-console --warn self-wakes,lost-waker,never-yielded --allow self-wakes,never-yielded`
The application panicked (crashed).
Message:  [LostWaker]
Location: tokio-console/src/main.rs:79

Copy link
Collaborator Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

tokio-console/src/config.rs Show resolved Hide resolved
@Rustin170506 Rustin170506 marked this pull request as ready for review February 7, 2024 16:17
@Rustin170506 Rustin170506 requested a review from a team as a code owner February 7, 2024 16:17
@Rustin170506 Rustin170506 changed the title WIP: feat: add --allow flag feat: add --allow flag Feb 7, 2024
Copy link
Collaborator

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, just a couple of small things.

tokio-console/src/config.rs Show resolved Hide resolved
tokio-console/src/main.rs Outdated Show resolved Hide resolved
tokio-console/src/config.rs Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

Copy link
Collaborator

@hds hds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Perhaps just add the comment on AllowedWarnings regarding ValueEnum that Eliza suggested and it's good to go!

Signed-off-by: hi-rustin <[email protected]>
@Rustin170506
Copy link
Collaborator Author

Perhaps just add the comment on AllowedWarnings regarding ValueEnum that Eliza suggested and it's good to go!

Added.
Thanks for your review! 💚 💙 💜 💛 ❤️

@hds
Copy link
Collaborator

hds commented Feb 12, 2024

Added.
Thanks for your review! 💚 💙 💜 💛 ❤️

Great, thank you! Would you mind adding a bit of a longer description to this MR (we'll then use that as the commit message).

Ideally 2 paragraphs: the first one explaining what the PR is trying to achieve (why are we making the change). The second paragraph should explain at a high level what is being changed.

This then becomes a really valuable resource for the future to understand why certain changes have been made.
Thanks!

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Please make sure to update the commit message the way @hds described, and then we'd love to merge this PR!

@Rustin170506
Copy link
Collaborator Author

Great, thank you! Would you mind adding a bit of a longer description to this MR (we'll then use that as the commit message).

Ideally 2 paragraphs: the first one explaining what the PR is trying to achieve (why are we making the change). The second paragraph should explain at a high level what is being changed.

Added! Thanks for your remainder. I will add it to all my PRs.

@hds hds merged commit 8da7037 into tokio-rs:main Feb 13, 2024
11 checks passed
hds pushed a commit that referenced this pull request Jun 10, 2024
…scriber-v0.3.0 (#557)

## 🤖 New release
* `tokio-console`: 0.1.10 -> 0.1.11
* `console-api`: 0.6.0 -> 0.7.0
* `console-subscriber`: 0.2.0 -> 0.3.0


## `tokio-console`

## tokio-console-v0.1.11 - (2024-06-10)

### Added

- Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa))
- Add flags and configurations for warnings ([#493](#493)) ([174883f](174883f))
- Add `--allow` flag ([#513](#513)) ([8da7037](8da7037))

### Documented

- Add note about running on Windows ([#510](#510)) ([a0d20fd](a0d20fd))

### Fixed

- Ignore key release events ([#468](#468)) ([715713a](715713a))
- Accept only `file://`, `http://`, `https://` URI ([#486](#486)) ([031bddd](031bddd))
- Fix column sorting in resources tab ([#491](#491)) ([96c65bd](96c65bd))
- Only trigger lints on async tasks ([#517](#517)) ([4593222](4593222))
- Remove duplicate controls from async ops view ([#519](#519)) ([f28ba4a](f28ba4a))
- Add pretty format for 'last woken' time ([#529](#529)) ([ea11ad8](ea11ad8))


## `console-api`

## console-api-v0.7.0 - (2024-06-10)

### <a id = "0.7.0-breaking"></a>Breaking Changes
- **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.10.x.

### Documented

- Fix typo in proto ([#472](#472)) ([2dd3559](2dd3559))

### Updated

- [**breaking**](#0.7.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c))


## `console-subscriber`

## console-subscriber-v0.3.0 - (2024-06-10)

### <a id = "0.3.0-breaking"></a>Breaking Changes
- **Bump tonic to 0.11 ([#547](#547 ([ef6816c](https://github.com/tokio-rs/console/commit/ef6816caa0fe84171105b513425506f25d3082af))<br />This is a breaking change for users of `console-api` and
`console-subscriber`, as it changes the public `tonic` dependency to a
semver-incompatible version. This breaks compatibility with `tonic`
0.10.x.

### Added

- Replace target column with kind column in tasks view ([#478](#478)) ([903d9fa](903d9fa))
- Reduce retention period to fit in max message size ([#503](#503)) ([bd3dd71](bd3dd71))
- Support grpc-web and add `grpc-web` feature ([#498](#498)) ([4150253](4150253))

### Documented

- Add a grpc-web app example ([#526](#526)) ([4af30f2](4af30f2))
- Fix typo in doc comment ([#543](#543)) ([ff22678](ff22678))

### Fixed

- Don't save poll_ops if no-one is receiving them ([#501](#501)) ([1656c79](1656c79))
- Ignore metadata that is not a span or event ([#554](#554)) ([852a977](852a977))

### Updated

- [**breaking**](#0.3.0-breaking) Bump tonic to 0.11 ([#547](#547)) ([ef6816c](ef6816c))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants